From bf06cad5d9d147fc9b5bc87d8166ead776f947f0 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Tue, 28 Jul 2020 16:41:44 +0100 Subject: [PATCH] a11y: Add proper error reporting to value collection We're currently overloading NULL to mean both "this value is undefined, and should be reset to its default" and "the value collection failed". Let's do error reporting right, by using GError to mean "the collection failed, for this specific reason"; then, we can use a NULL return value to signal that the accessible attribute should be reset to its default value. This is only relevant for pointer-sized attribute values: strings, references, and reference lists; numeric, boolean, tristate, and token values either cannot be undefined, or have a specific "undefined" value. --- gtk/gtkaccessible.c | 116 ++++++++++++----- gtk/gtkaccessiblevalue.c | 218 +++++++++++++++++++++++--------- gtk/gtkaccessiblevalueprivate.h | 12 +- gtk/gtktestatcontext.c | 21 ++- 4 files changed, 272 insertions(+), 95 deletions(-) diff --git a/gtk/gtkaccessible.c b/gtk/gtkaccessible.c index 850077b468..fcaa6edb58 100644 --- a/gtk/gtkaccessible.c +++ b/gtk/gtkaccessible.c @@ -144,13 +144,23 @@ gtk_accessible_update_state (GtkAccessible *self, while (state != -1) { - GtkAccessibleValue *value = gtk_accessible_value_collect_for_state (state, &args); - - if (value == NULL) - goto out; + GError *error = NULL; + GtkAccessibleValue *value = + gtk_accessible_value_collect_for_state (state, &error, &args); + + if (error != NULL) + { + g_critical ("Unable to collect value for state “%s”: %s", + gtk_accessible_state_get_attribute_name (state), + error->message); + g_error_free (error); + goto out; + } gtk_at_context_set_accessible_state (context, state, value); - gtk_accessible_value_unref (value); + + if (value != NULL) + gtk_accessible_value_unref (value); state = va_arg (args, int); } @@ -187,14 +197,24 @@ gtk_accessible_update_state_value (GtkAccessible *self, if (context == NULL) return; + GError *error = NULL; GtkAccessibleValue *real_value = - gtk_accessible_value_collect_for_state_value (state, value); + gtk_accessible_value_collect_for_state_value (state, value, &error); - if (real_value == NULL) - return; + if (error != NULL) + { + g_critical ("Unable to collect the value for state “%s”: %s", + gtk_accessible_state_get_attribute_name (state), + error->message); + g_error_free (error); + return; + } gtk_at_context_set_accessible_state (context, state, real_value); - gtk_accessible_value_unref (real_value); + + if (real_value != NULL) + gtk_accessible_value_unref (real_value); + gtk_at_context_update (context); } @@ -239,14 +259,23 @@ gtk_accessible_update_property (GtkAccessible *self, while (property != -1) { - GtkAccessibleValue *value = gtk_accessible_value_collect_for_property (property, &args); - - /* gtk_accessible_value_collect_for_property() will warn for us */ - if (value == NULL) - goto out; + GError *error = NULL; + GtkAccessibleValue *value = + gtk_accessible_value_collect_for_property (property, &error, &args); + + if (error != NULL) + { + g_critical ("Unable to collect the value for property “%s”: %s", + gtk_accessible_property_get_attribute_name (property), + error->message); + g_error_free (error); + goto out; + } gtk_at_context_set_accessible_property (context, property, value); - gtk_accessible_value_unref (value); + + if (value != NULL) + gtk_accessible_value_unref (value); property = va_arg (args, int); } @@ -283,14 +312,24 @@ gtk_accessible_update_property_value (GtkAccessible *self, if (context == NULL) return; + GError *error = NULL; GtkAccessibleValue *real_value = - gtk_accessible_value_collect_for_property_value (property, value); + gtk_accessible_value_collect_for_property_value (property, value, &error); - if (real_value == NULL) - return; + if (error != NULL) + { + g_critical ("Unable to collect the value for property “%s”: %s", + gtk_accessible_property_get_attribute_name (property), + error->message); + g_error_free (error); + return; + } gtk_at_context_set_accessible_property (context, property, real_value); - gtk_accessible_value_unref (real_value); + + if (real_value != NULL) + gtk_accessible_value_unref (real_value); + gtk_at_context_update (context); } @@ -326,14 +365,23 @@ gtk_accessible_update_relation (GtkAccessible *self, while (relation != -1) { - GtkAccessibleValue *value = gtk_accessible_value_collect_for_relation (relation, &args); - - /* gtk_accessible_value_collect_for_relation() will warn for us */ - if (value == NULL) - goto out; + GError *error = NULL; + GtkAccessibleValue *value = + gtk_accessible_value_collect_for_relation (relation, &error, &args); + + if (error != NULL) + { + g_critical ("Unable to collect the value for relation “%s”: %s", + gtk_accessible_relation_get_attribute_name (relation), + error->message); + g_error_free (error); + goto out; + } gtk_at_context_set_accessible_relation (context, relation, value); - gtk_accessible_value_unref (value); + + if (value != NULL) + gtk_accessible_value_unref (value); relation = va_arg (args, int); } @@ -370,13 +418,23 @@ gtk_accessible_update_relation_value (GtkAccessible *self, if (context == NULL) return; + GError *error = NULL; GtkAccessibleValue *real_value = - gtk_accessible_value_collect_for_relation_value (relation, value); + gtk_accessible_value_collect_for_relation_value (relation, value, &error); - if (real_value == NULL) - return; + if (error != NULL) + { + g_critical ("Unable to collect the value for relation “%s”: %s", + gtk_accessible_relation_get_attribute_name (relation), + error->message); + g_error_free (error); + return; + } gtk_at_context_set_accessible_relation (context, relation, real_value); - gtk_accessible_value_unref (real_value); + + if (real_value != NULL) + gtk_accessible_value_unref (real_value); + gtk_at_context_update (context); } diff --git a/gtk/gtkaccessiblevalue.c b/gtk/gtkaccessiblevalue.c index 2921d99907..5ea1db0488 100644 --- a/gtk/gtkaccessiblevalue.c +++ b/gtk/gtkaccessiblevalue.c @@ -47,6 +47,9 @@ #include "gtkaccessible.h" #include "gtkenums.h" +#include +#include + G_DEFINE_QUARK (gtk-accessible-value-error-quark, gtk_accessible_value_error) G_DEFINE_BOXED_TYPE (GtkAccessibleValue, gtk_accessible_value, @@ -931,8 +934,9 @@ gtk_accessible_value_get_default_for_state (GtkAccessibleState state) } static GtkAccessibleValue * -gtk_accessible_value_collect_valist (const GtkAccessibleCollect *cstate, - va_list *args) +gtk_accessible_value_collect_valist (const GtkAccessibleCollect *cstate, + GError **error, + va_list *args) { GtkAccessibleValue *res = NULL; GtkAccessibleCollectType ctype = cstate->ctype; @@ -970,7 +974,7 @@ gtk_accessible_value_collect_valist (const GtkAccessibleCollect *cstate, { int value = va_arg (*args, int); - if (value == GTK_ACCESSIBLE_VALUE_UNDEFINED) + if (collects_undef && value == GTK_ACCESSIBLE_VALUE_UNDEFINED) res = gtk_undefined_accessible_value_new (); else res = gtk_tristate_accessible_value_new (value); @@ -995,6 +999,14 @@ gtk_accessible_value_collect_valist (const GtkAccessibleCollect *cstate, res = (* ctor) (value); } + + if (res == NULL) + { + g_set_error (error, GTK_ACCESSIBLE_VALUE_ERROR, + GTK_ACCESSIBLE_VALUE_ERROR_INVALID_TOKEN, + "Invalid value for token attribute: %d", + value); + } } break; @@ -1019,6 +1031,14 @@ gtk_accessible_value_collect_valist (const GtkAccessibleCollect *cstate, double value = va_arg (*args, double); + if (isnan (value) || isinf (value)) + { + g_set_error_literal (error, GTK_ACCESSIBLE_VALUE_ERROR, + GTK_ACCESSIBLE_VALUE_ERROR_INVALID_VALUE, + "Invalid numeric value"); + return NULL; + } + if (ctor == NULL) res = gtk_number_accessible_value_new (value); else @@ -1034,9 +1054,14 @@ gtk_accessible_value_collect_valist (const GtkAccessibleCollect *cstate, const char *value = va_arg (*args, char*); if (ctor == NULL) - res = gtk_string_accessible_value_new (value); + { + if (value != NULL) + res = gtk_string_accessible_value_new (value); + } else - res = (* ctor) (value); + { + res = (* ctor) (value); + } } break; @@ -1047,19 +1072,22 @@ gtk_accessible_value_collect_valist (const GtkAccessibleCollect *cstate, gpointer value = va_arg (*args, gpointer); - if (value == NULL) + if (value != NULL && !GTK_IS_ACCESSIBLE (value)) { - if (ctor == NULL) - res = gtk_undefined_accessible_value_new (); - else - res = (* ctor) (value); + g_set_error_literal (error, GTK_ACCESSIBLE_VALUE_ERROR, + GTK_ACCESSIBLE_VALUE_ERROR_INVALID_VALUE, + "Reference does not implement GtkAccessible"); + return NULL; } - else + + if (ctor == NULL) { - if (ctor == NULL) + if (value != NULL) res = gtk_reference_accessible_value_new (value); - else - res = (* ctor) (value); + } + else + { + res = (* ctor) (value); } } break; @@ -1071,19 +1099,16 @@ gtk_accessible_value_collect_valist (const GtkAccessibleCollect *cstate, GList *value = va_arg (*args, gpointer); - if (value == NULL) + if (ctor == NULL) { - if (ctor == NULL) - res = gtk_undefined_accessible_value_new (); + if (value == NULL) + res = NULL; else - res = (* ctor) (value); + res = gtk_reference_list_accessible_value_new (value); } else { - if (ctor == NULL) - res = gtk_reference_list_accessible_value_new (value); - else - res = (* ctor) (value); + res = (* ctor) (value); } } break; @@ -1091,7 +1116,7 @@ gtk_accessible_value_collect_valist (const GtkAccessibleCollect *cstate, case GTK_ACCESSIBLE_COLLECT_UNDEFINED: case GTK_ACCESSIBLE_COLLECT_INVALID: default: - g_critical ("Unknown type for accessible state “%s”", cstate->name); + g_assert_not_reached (); break; } @@ -1099,8 +1124,9 @@ gtk_accessible_value_collect_valist (const GtkAccessibleCollect *cstate, } static GtkAccessibleValue * -gtk_accessible_value_collect_value (const GtkAccessibleCollect *cstate, - const GValue *value_) +gtk_accessible_value_collect_value (const GtkAccessibleCollect *cstate, + const GValue *value_, + GError **error) { GtkAccessibleValue *res = NULL; GtkAccessibleCollectType ctype = cstate->ctype; @@ -1138,7 +1164,7 @@ gtk_accessible_value_collect_value (const GtkAccessibleCollect *cstate, { int value = g_value_get_int (value_); - if (value == GTK_ACCESSIBLE_VALUE_UNDEFINED) + if (collects_undef && value == GTK_ACCESSIBLE_VALUE_UNDEFINED) res = gtk_undefined_accessible_value_new (); else res = gtk_tristate_accessible_value_new (value); @@ -1163,6 +1189,14 @@ gtk_accessible_value_collect_value (const GtkAccessibleCollect *cstate, res = (* ctor) (value); } + + if (res == NULL) + { + g_set_error (error, GTK_ACCESSIBLE_VALUE_ERROR, + GTK_ACCESSIBLE_VALUE_ERROR_INVALID_TOKEN, + "Invalid value for token attribute: %d", + value); + } } break; @@ -1187,6 +1221,14 @@ gtk_accessible_value_collect_value (const GtkAccessibleCollect *cstate, double value = g_value_get_double (value_); + if (isnan (value) || isinf (value)) + { + g_set_error_literal (error, GTK_ACCESSIBLE_VALUE_ERROR, + GTK_ACCESSIBLE_VALUE_ERROR_INVALID_VALUE, + "Invalid numeric value"); + return NULL; + } + if (ctor == NULL) res = gtk_number_accessible_value_new (value); else @@ -1202,9 +1244,14 @@ gtk_accessible_value_collect_value (const GtkAccessibleCollect *cstate, const char *value = g_value_get_string (value_); if (ctor == NULL) - res = gtk_string_accessible_value_new (value); + { + if (value != NULL) + res = gtk_string_accessible_value_new (value); + } else - res = (* ctor) (value); + { + res = (* ctor) (value); + } } break; @@ -1215,10 +1262,23 @@ gtk_accessible_value_collect_value (const GtkAccessibleCollect *cstate, gpointer value = g_value_get_object (value_); + if (value != NULL && !GTK_IS_ACCESSIBLE (value)) + { + g_set_error_literal (error, GTK_ACCESSIBLE_VALUE_ERROR, + GTK_ACCESSIBLE_VALUE_ERROR_INVALID_VALUE, + "Reference does not implement GtkAccessible"); + return NULL; + } + if (ctor == NULL) - res = gtk_reference_accessible_value_new (value); + { + if (value != NULL) + res = gtk_reference_accessible_value_new (value); + } else - res = (* ctor) (value); + { + res = (* ctor) (value); + } } break; @@ -1230,16 +1290,21 @@ gtk_accessible_value_collect_value (const GtkAccessibleCollect *cstate, GList *value = g_value_get_pointer (value_); if (ctor == NULL) - res = gtk_reference_list_accessible_value_new (value); + { + if (value != NULL) + res = gtk_reference_list_accessible_value_new (value); + } else - res = (* ctor) (value); + { + res = (* ctor) (value); + } } break; case GTK_ACCESSIBLE_COLLECT_UNDEFINED: case GTK_ACCESSIBLE_COLLECT_INVALID: default: - g_critical ("Unknown type for accessible state “%s”", cstate->name); + g_assert_not_reached (); break; } @@ -1249,43 +1314,57 @@ gtk_accessible_value_collect_value (const GtkAccessibleCollect *cstate, /*< private > * gtk_accessible_value_collect_for_state: * @state: a #GtkAccessibleState + * @error: return location for a #GError * @args: a `va_list` reference * * Collects and consumes the next item in the @args variadic arguments list, * and returns a #GtkAccessibleValue for it. * - * Returns: (transfer full): a #GtkAccessibleValue + * If the collection fails, @error is set and %NULL is returned. + * + * The returned value could be %NULL even on success, in which case the state + * should be reset to its default value by the caller. + * + * Returns: (transfer full) (nullable): a #GtkAccessibleValue */ GtkAccessibleValue * -gtk_accessible_value_collect_for_state (GtkAccessibleState state, - va_list *args) +gtk_accessible_value_collect_for_state (GtkAccessibleState state, + GError **error, + va_list *args) { const GtkAccessibleCollect *cstate = &collect_states[state]; g_return_val_if_fail (state <= GTK_ACCESSIBLE_STATE_SELECTED, NULL); - return gtk_accessible_value_collect_valist (cstate, args); + return gtk_accessible_value_collect_valist (cstate, error, args); } /*< private > * gtk_accessible_value_collect_for_state_value: * @state: a #GtkAccessibleState * @value: a #GValue + * @error: return location for a #GError * * Retrieves the value stored inside @value and returns a #GtkAccessibleValue * for the given @state. * - * Returns: (transfer full): a #GtkAccessibleValue + * If the collection fails, @error is set and %NULL is returned. + * + * The returned value could be %NULL even on success, in which case the state + * should be reset to its default value by the caller. + * + * Returns: (transfer full) (nullable): a #GtkAccessibleValue */ GtkAccessibleValue * -gtk_accessible_value_collect_for_state_value (GtkAccessibleState state, - const GValue *value) +gtk_accessible_value_collect_for_state_value (GtkAccessibleState state, + const GValue *value, + GError **error) { const GtkAccessibleCollect *cstate = &collect_states[state]; g_return_val_if_fail (state <= GTK_ACCESSIBLE_STATE_SELECTED, NULL); - return gtk_accessible_value_collect_value (cstate, value); + return gtk_accessible_value_collect_value (cstate, value, error); } /*< private > @@ -1355,43 +1434,54 @@ gtk_accessible_value_get_default_for_property (GtkAccessibleProperty property) /*< private > * gtk_accessible_value_collect_for_property: * @property: a #GtkAccessibleProperty + * @error: return location for a #GError * @args: a `va_list` reference * * Collects and consumes the next item in the @args variadic arguments list, * and returns a #GtkAccessibleValue for it. * - * Returns: (transfer full): a #GtkAccessibleValue + * If the collection fails, @error is set. + * + * Returns: (transfer full) (nullable): a #GtkAccessibleValue */ GtkAccessibleValue * -gtk_accessible_value_collect_for_property (GtkAccessibleProperty property, - va_list *args) +gtk_accessible_value_collect_for_property (GtkAccessibleProperty property, + GError **error, + va_list *args) { const GtkAccessibleCollect *cstate = &collect_props[property]; g_return_val_if_fail (property <= GTK_ACCESSIBLE_PROPERTY_VALUE_TEXT, NULL); - return gtk_accessible_value_collect_valist (cstate, args); + return gtk_accessible_value_collect_valist (cstate, error, args); } /*< private > * gtk_accessible_value_collect_for_property_value: * @property: a #GtkAccessibleProperty * @value: a #GValue + * @error: return location for a #GError * * Retrieves the value stored inside @value and returns a #GtkAccessibleValue * for the given @property. * - * Returns: (transfer full): a #GtkAccessibleValue + * If the collection fails, @error is set. + * + * The returned value could be %NULL even on success, in which case the property + * should be reset to its default value by the caller. + * + * Returns: (transfer full) (nullable): a #GtkAccessibleValue */ GtkAccessibleValue * -gtk_accessible_value_collect_for_property_value (GtkAccessibleProperty property, - const GValue *value) +gtk_accessible_value_collect_for_property_value (GtkAccessibleProperty property, + const GValue *value, + GError **error) { const GtkAccessibleCollect *cstate = &collect_props[property]; g_return_val_if_fail (property <= GTK_ACCESSIBLE_PROPERTY_VALUE_TEXT, NULL); - return gtk_accessible_value_collect_value (cstate, value); + return gtk_accessible_value_collect_value (cstate, value, error); } /*< private > @@ -1450,43 +1540,57 @@ gtk_accessible_value_get_default_for_relation (GtkAccessibleRelation relation) /*< private > * gtk_accessible_value_collect_for_relation: * @relation: a #GtkAccessibleRelation + * @error: return location for a #GError * @args: a `va_list` reference * * Collects and consumes the next item in the @args variadic arguments list, * and returns a #GtkAccessibleValue for it. * - * Returns: (transfer full): a #GtkAccessibleValue + * If the collection fails, @error is set and %NULL is returned. + * + * The returned value could be %NULL even on success, in which case the relation + * should be reset to its default value by the caller. + * + * Returns: (transfer full) (nullable): a #GtkAccessibleValue */ GtkAccessibleValue * -gtk_accessible_value_collect_for_relation (GtkAccessibleRelation relation, - va_list *args) +gtk_accessible_value_collect_for_relation (GtkAccessibleRelation relation, + GError **error, + va_list *args) { const GtkAccessibleCollect *cstate = &collect_rels[relation]; g_return_val_if_fail (relation <= GTK_ACCESSIBLE_RELATION_SET_SIZE, NULL); - return gtk_accessible_value_collect_valist (cstate, args); + return gtk_accessible_value_collect_valist (cstate, error, args); } /*< private > * gtk_accessible_value_collect_for_relation_value: * @relation: a #GtkAccessibleRelation * @value: a #GValue + * @error: return location for a #GError * * Retrieves the value stored inside @value and returns a #GtkAccessibleValue * for the given @relation. * - * Returns: (transfer full): a #GtkAccessibleValue + * If the collection fails, @error is set and %NULL is returned. + * + * The returned value could be %NULL even on success, in which case the relation + * should be reset to its default value by the caller. + * + * Returns: (transfer full) (nullable): a #GtkAccessibleValue */ GtkAccessibleValue * -gtk_accessible_value_collect_for_relation_value (GtkAccessibleRelation relation, - const GValue *value) +gtk_accessible_value_collect_for_relation_value (GtkAccessibleRelation relation, + const GValue *value, + GError **error) { const GtkAccessibleCollect *cstate = &collect_rels[relation]; g_return_val_if_fail (relation <= GTK_ACCESSIBLE_RELATION_SET_SIZE, NULL); - return gtk_accessible_value_collect_value (cstate, value); + return gtk_accessible_value_collect_value (cstate, value, error); } /* }}} */ diff --git a/gtk/gtkaccessiblevalueprivate.h b/gtk/gtkaccessiblevalueprivate.h index 9405c872ef..073b2c2c45 100644 --- a/gtk/gtkaccessiblevalueprivate.h +++ b/gtk/gtkaccessiblevalueprivate.h @@ -93,21 +93,27 @@ gboolean gtk_accessible_value_equal (const G GtkAccessibleValue * gtk_accessible_value_get_default_for_state (GtkAccessibleState state); GtkAccessibleValue * gtk_accessible_value_collect_for_state (GtkAccessibleState state, + GError **error, va_list *args); GtkAccessibleValue * gtk_accessible_value_collect_for_state_value (GtkAccessibleState state, - const GValue *value); + const GValue *value, + GError **error); GtkAccessibleValue * gtk_accessible_value_get_default_for_property (GtkAccessibleProperty property); GtkAccessibleValue * gtk_accessible_value_collect_for_property (GtkAccessibleProperty property, + GError **error, va_list *args); GtkAccessibleValue * gtk_accessible_value_collect_for_property_value (GtkAccessibleProperty property, - const GValue *value); + const GValue *value, + GError **error); GtkAccessibleValue * gtk_accessible_value_get_default_for_relation (GtkAccessibleRelation relation); GtkAccessibleValue * gtk_accessible_value_collect_for_relation (GtkAccessibleRelation relation, + GError **error, va_list *args); GtkAccessibleValue * gtk_accessible_value_collect_for_relation_value (GtkAccessibleRelation relation, - const GValue *value); + const GValue *value, + GError **error); /* Basic values */ GtkAccessibleValue * gtk_undefined_accessible_value_new (void); diff --git a/gtk/gtktestatcontext.c b/gtk/gtktestatcontext.c index f6c401b339..ff0b6833fd 100644 --- a/gtk/gtktestatcontext.c +++ b/gtk/gtktestatcontext.c @@ -161,13 +161,16 @@ gtk_test_accessible_check_property (GtkAccessible *accessible, va_start (args, property); + GError *error = NULL; GtkAccessibleValue *check_value = - gtk_accessible_value_collect_for_property (property, &args); + gtk_accessible_value_collect_for_property (property, &error, &args); va_end (args); + g_assert_no_error (error); + if (check_value == NULL) - return g_strdup ("undefined"); + check_value = gtk_accessible_value_get_default_for_property (property); GtkATContext *context = gtk_accessible_get_at_context (accessible); GtkAccessibleValue *real_value = @@ -220,13 +223,16 @@ gtk_test_accessible_check_state (GtkAccessible *accessible, va_start (args, state); + GError *error = NULL; GtkAccessibleValue *check_value = - gtk_accessible_value_collect_for_state (state, &args); + gtk_accessible_value_collect_for_state (state, &error, &args); va_end (args); + g_assert_no_error (error); + if (check_value == NULL) - return g_strdup ("undefined"); + check_value = gtk_accessible_value_get_default_for_state (state); GtkATContext *context = gtk_accessible_get_at_context (accessible); GtkAccessibleValue *real_value = @@ -279,13 +285,16 @@ gtk_test_accessible_check_relation (GtkAccessible *accessible, va_start (args, relation); + GError *error = NULL; GtkAccessibleValue *check_value = - gtk_accessible_value_collect_for_relation (relation, &args); + gtk_accessible_value_collect_for_relation (relation, &error, &args); va_end (args); + g_assert_no_error (error); + if (check_value == NULL) - return g_strdup ("undefined"); + check_value = gtk_accessible_value_get_default_for_relation (relation); GtkATContext *context = gtk_accessible_get_at_context (accessible); GtkAccessibleValue *real_value = -- 2.30.2